Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NickAkhmetov/CAT-751 Prevent full-page errors from malformed provenance data #3560

Merged

Conversation

NickAkhmetov
Copy link
Collaborator

Summary

This donor's provenance data is incomplete (it is missing the edges that connect activities to their parent entities): https://portal.hubmapconsortium.org/browse/donor/a0bdb4249eb3a6bd1865f948875c2f96

Removing the required status from that key does not resolve the issue, since these edges are mandatory to be able to draw the graph.

I have escalated the underlying data issue to Zhou in a private message; in the meantime, we can improve the way this error is handled on the front-end by preventing the triggering of the full-page error boundary and interrupting the user's browsing session.

I used the same DetailsAccordion component we use to report Vitessce visualization errors to be consistent.

Design Documentation/Original Tickets

https://hms-dbmi.atlassian.net/browse/CAT-751

Testing

The problematic donor page has been visited with the dev server configured to suppress exception overlays to confirm that only the appropriate error handling is included.

Screenshots/Video

image

Checklist

  • Code follows the project's coding standards
    • Lint checks pass locally
    • New CHANGELOG-your-feature-name-here.md is present in the root directory, describing the change(s) in full sentences.
  • Unit tests covering the new feature have been added
  • All existing tests pass
  • Any relevant documentation in JIRA/Confluence has been updated to reflect the new feature
  • Any new functionalities have appropriate analytics functionalities added

Additional Notes

Should we consider removing the webpack dev server error overlay in general? It seems counterproductive since it takes up the full page, which errors not caught by a lower-level error boundary do as well; not having this overlay might encourage us to add targeted boundaries such as this.

john-conroy
john-conroy previously approved these changes Oct 7, 2024
<Stack p={4}>
<Typography variant="subtitle1">An error occurred while attempting to display the provenance graph.</Typography>
<DetailsAccordion summary="Click to expand error details">
<div>{error?.message}</div>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this div be a Typography instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll swap it in and use body2 👍🏻

@NickAkhmetov NickAkhmetov merged commit 60d5a49 into main Oct 7, 2024
8 checks passed
@NickAkhmetov NickAkhmetov deleted the nickakhmetov/cat-751-provenance-graph-error-boundary branch October 7, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants